Skip to content

Add SFR Box integration#84780

Merged
epenet merged 9 commits into
home-assistant:devfrom
epenet:sfr_box
Dec 31, 2022
Merged

Add SFR Box integration#84780
epenet merged 9 commits into
home-assistant:devfrom
epenet:sfr_box

Conversation

@epenet
Copy link
Copy Markdown
Contributor

@epenet epenet commented Dec 29, 2022

Proposed change

Add SFR Box (broadband router) integration

Minimal integration with:

  • config flow
  • DSL sensors
  • device info (to group the sensors)

Link to the library: https://github.com/hacf-fr/sfrbox-api

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

Copy link
Copy Markdown
Member

@Kane610 Kane610 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@epenet epenet merged commit 896526c into home-assistant:dev Dec 31, 2022
@epenet
Copy link
Copy Markdown
Contributor Author

epenet commented Dec 31, 2022

Thanks @Kane610 👍

@epenet epenet deleted the sfr_box branch December 31, 2022 09:14
Comment on lines +115 to +120
"No Defect",
"Of Frame",
"Loss Of Signal",
"Loss Of Power",
"Loss Of Signal Quality",
"Unknown",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be lower case and been put into translations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #84977

Comment on lines +130 to +139
"Idle",
"G.994 Training",
"G.992 Started",
"G.922 Channel Analysis",
"G.992 Message Exchange",
"G.993 Started",
"G.993 Channel Analysis",
"G.993 Message Exchange",
"Showtime",
"Unknown",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #84977

SFRBoxSensor(data["dsl_coordinator"], description, system_info)
for description in SENSOR_TYPES
]
async_add_entities(entities, True)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a coordinator that already has data, the True parameter seems unneeded here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The coordinator doesn't yet have data, at is it not (currently) initialised on setup.
I will address this when I add the second coordinator.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #85039

Comment thread homeassistant/components/sfr_box/sensor.py
Comment on lines +21 to +26
try:
system_info = await box.system_get_info()
except SFRBoxError as err:
raise ConfigEntryNotReady(
f"Unable to connect to {entry.data[CONF_HOST]}"
) from err
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The coordinator can already handle this on first request/refresh,thispart is thus not needed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

system_get_info is only called in setup - not via a coordinator
I will address this when I add "system" sensors.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #85039


async def _async_update_data(self) -> DslInfo:
"""Update data."""
return await self._box.dsl_get_info()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can throw an SFRBoxError, which is unhandled?

Copy link
Copy Markdown
Contributor Author

@epenet epenet Jan 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #84977 #85039

@github-actions github-actions Bot locked and limited conversation to collaborators Jan 1, 2023
Comment thread homeassistant/components/sfr_box/config_flow.py
Comment thread homeassistant/components/sfr_box/config_flow.py
Comment thread homeassistant/components/sfr_box/__init__.py
Comment thread homeassistant/components/sfr_box/sensor.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants